-
Notifications
You must be signed in to change notification settings - Fork 432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for language level errors (LangError
)
#1450
Conversation
We're gonna try and handle an ink! error later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach looks good, I think a unification of the LangError
and DispathError
is a good idea if you didn't already have that planned.
@HCastano Can we use Also CC @ascjones |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ the comprehensive e2e tests
#[ink(message)] | ||
pub fn flip_check(&mut self) { | ||
self.flipper | ||
.flip_checked() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would testing err_flip_checked
be useful do you think? As in it will be a Ok(Err(()))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be useful, but maybe in the context of a different set of tests around error handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding an error for NotPayable
could be quite easy? Can be a follow up but calling return_value
instead of using ?
seems easy enough.
Apart from that it looks good to me. I compiled some simple contracts and the metadata looks sensible.
DispatchError
LangError
)
Yeah now that this is in place it should be almost trivial to add that. Figured it would be better left for a follow-up though |
I think “unchecked” fits more for methods that return |
Most idiomatic would be using |
Implements the Message related changes of #1207.
The idea here is that every message now returns a
Result<Message::Output, LangError>
,where a
LangError
is a type of error which doesn't originate from the contract itself,nor from the underlying execution environment (so the Contracts pallet in this case).
An example of where this would arise is if a caller tries to use a non-existent message
selector for a contract. Previously, the contract would trap and not allow the caller to
do any sort of error handling if it encountered a non-existent selector.
This breaks the ABI in two ways: first, all contract messages now have a
Result
returntype, and second a new field,
lang_err
, will be introduced as part of the contractspec. The second change allows other languages, such as Solang, to use an equivalent
LangError
.If you're curious, click here for a snippet of the new metadata for the Flipper contract.
TODO:
LangError
type to metadataLangError
sTODOs in Follow-Ups:
LangError
from instantiate #1512)LangError
CallBuilder
API not require an explicitResult
(Clean upCallBuilder
return()
type #1525)cc @xgreenx since you were the original poster for the issue